Repository navigation
tools: skip running test-shared on deps changes - #60433
Conversation
|
Review requested:
|
| - deps/icu-tmp/** | ||
| - deps/llhttp/** | ||
| - deps/nghttp2/** | ||
| - deps/ngtcp2/** |
There was a problem hiding this comment.
npm is not a shared library but it is not bundled into node binary as well.
| - deps/ngtcp2/** | |
| - deps/ngtcp2/** | |
| - deps/npm/** |
There was a problem hiding this comment.
But wouldn't it be better to still check npm still works when Node.js is built with shared libraries?
There was a problem hiding this comment.
I'm fine either way, both of you are making valid points
There was a problem hiding this comment.
@legendecas can you clarify whether this is a blocking concern?
There was a problem hiding this comment.
Ah, sorry, this is not a blocking concern. I was trying to understand if it can be optimized to exclude npm as well.
joyeecheung
left a comment
There was a problem hiding this comment.
I don't think we need to run it on npm upgrades, or I think the npm tests we have in core don't really cover enough of npm for this, but I'm also fine with just giving it a try - if it leads to whatever issue we can always just add it to the ignore list.
|
Landed in 515ca29 |
PR-URL: #60433 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Running
test-shared.ymlfor changes on folder that it doesn't use is pointless, it can't affect it as those do not even make it to the slim tarball it uses:node/Makefile
Lines 1221 to 1239 in 6176222